-
Notifications
You must be signed in to change notification settings - Fork 190
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[sensor_msgs] add PanoramaInfo.msg and update CMakeLists.txt #171
base: noetic-devel
Are you sure you want to change the base?
Conversation
add PanoramaInfo on jsk_recognition_msgs until ros/common_msgs#171 merged
…r_msgs/PanoramaInfo" This reverts commit 6fd5dae , until ros/common_msgs#171 has merged
Hi, we are also using panoramic images in our projects. However, we fuse them from 4-6 90° FOV cameras, so our perspective is a bit different. What I think could be fruitful is predefining an enum of possible projections (maybe take inspiration in Hugin?), with a field that could hold any custom info if e.g. Also, the message definition should explicitly specify what should a 360° pano have in Last, there is also the question whether a LUT (lookup table) should not be a part of this message (or an accompanying meta info message). Are you sure that given just the image, angular bounds and name of the projection you are able to construct the unprojection function? Maybe @Martin-Oehler could be interested in this discussion as he is the author of https://github.com/tu-darmstadt-ros-pkg/image_projection . |
Hello everyone, However, I share the sentiment of @peci1, that the message is too specific. Most projections have some kind of parameters. Even if you limit "Panorama projections" to cylindrical projections, you would also need the radius.
I'm not sure, if this would be helpful. This makes it necessary to edit the message if you want to add a new projection type and the message can no longer be used for custom applications/projections. I think, we can take inspiration from the similar sensor_msgs/CameraInfo where the distortion model is just a string and parameters are a vector of doubles.
Yes, agreed.
Assuming, that the projection has been created by some kind of analytical function, I don't think, this is necessary. But there should definitely be a way to add additional parameters. Furthermore, the current definition does not include image dimensions. Similar to CameraInfo, I think this is important information. Next, I would also add a pose to specify the projection center. Since projections can be created on-demand, the projection center does not necessarily coincide with an existing camera frame. Lastly, I would like to raise the question, if a specific PanoramaInfo message is even necessary. Considering the message fields, it would be very similar to a potential more general ProjectionInfo that could also represent other projections like stereographic. |
What I meant by enum was actually the .msg "enum", which is just a bunch of constants with shared prefix. It should probably be a string field with a few predefined constants that can provide its value. The idea was to have a unified set of basic projections where the name that is in the projection type parameter would be unambiguous. The way it is currently done in CameraInfo is weird, as it is a string field, but to find the possible basic values, you have to look in the C++ header file somewhere else in the project. An example could look like this:
You're right that if you have the function definition, you can quite easily build the LUT. The idea is that to improve "generality" of bagfiles containing these images, if there were a LUT recorded, the user replaying the bag would not need to care about the particular projection function and would easily get a correctly projected panorama even for custom functions. I agree that a LUT can be quite large, so it would either deserve its own message type (and could be published as a latched topic alongside the panorama image), or it could be "hijacked" into an
Agree.
Sounds interesting. Would you be able to sketch a definition of such message? |
Yes, I guess that approach makes sense. I don't like having the constants in a separate cpp either.
That's an interesting idea, but should probably be a separate message. You could use
Well, I guess it would be pretty similar to this PR, something like:
|
|
Add PanoramaInfo.msg to represent meta-information about panoramic images.
We are using panoramic images generated from fisher eye images.
And we are defining a meta-information message of these panoramic images. jsk-ros-pkg/jsk_recognition#2579
This meta-information is necessary if calculating geometric information from panoramic images.
And I think this is useful for other users of panorama images.
PanoramaInfo.msg contains information below
CC: @k-okada